feat(cel): enable string and list extension functions in CEL expressions#2725
Conversation
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2725 +/- ##
=======================================
Coverage 59.46% 59.46%
=======================================
Files 209 209
Lines 20699 20702 +3
=======================================
+ Hits 12308 12311 +3
Misses 7616 7616
Partials 775 775 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enables CEL string and list extensions to support operations like join, replace, and substring, and includes corresponding unit and E2E tests. The reviewer noted that the go.mod file contains redundant direct dependencies on both v84 and v85 of go-github and requested that the test failure message modification be reverted to maintain helpful diff output.
| github.com/google/cel-go v0.28.0 | ||
| github.com/google/go-cmp v0.7.0 | ||
| github.com/google/go-github/scrape v0.0.0-20260403152401-96a365122246 | ||
| github.com/google/go-github/v84 v84.0.0 |
There was a problem hiding this comment.
Having both v84 and v85 of github.com/google/go-github as direct dependencies is redundant and can lead to dependency conflicts. Please remove the unused version.
| got := ReplacePlaceHoldersVariables(tt.template, tt.dicto, tt.rawEvent, tt.headers, tt.changedFiles) | ||
| if d := cmp.Diff(got, tt.expected); d != "" { | ||
| t.Fatalf("-got, +want: %v", d) | ||
| t.Fatalf("-got %s, +want: %s", got, tt.expected) |
There was a problem hiding this comment.
The test failure message was modified to remove the diff output (d). Using cmp.Diff and printing the diff is standard practice for test debugging and should be preserved.
| t.Fatalf("-got %s, +want: %s", got, tt.expected) | |
| t.Fatalf("-got, +want: %v", d) |
theakshaypant
left a comment
theakshaypant
left a comment
There was a problem hiding this comment.
LGTM.
Adds support for the built in cel methods for strings and lists by adding them to pac env.
Added unit test shows how/what are these new functions.
e2e test shows it working in an end to end flow.
zakisk
force-pushed
the
SRVKP-11940-support-string-ops-in-cel
branch
from
1aa9629 to
403f143
Compare
Previously, CEL expressions in Pipelines-as-Code only had access to
the core CEL operators, which limited users to basic comparisons and
logical expressions. Functions like join(), replace(), substring(),
and other string/list manipulation operations were unavailable,
forcing users to work around these limitations.
This adds cel-go's ext.Strings() and ext.Lists() extensions to the
CEL environment in pkg/cel/cel.go. This unlocks the full set of
standard CEL string operations (join, replace, substring, split,
trim, upperAscii, lowerAscii, etc.) and list operations, which can
be used in both `on-cel-expression` annotations and `{{ cel: }}`
template expressions.
Changes:
- pkg/cel/cel.go: Register ext.Strings() and ext.Lists() extensions
on the CEL environment alongside existing variable declarations
- pkg/templates/templating_test.go: Add unit tests for join(),
replace(), and substring() via the template placeholder path;
improve test failure message to show actual vs expected values
- test/testdata/pipelinerun-cel-string-join.yaml: New e2e test
fixture that uses files.all.join(", ") in a cel: template
- test/github_pullrequest_test.go: Add GHE e2e test
(TestGithubGHEPullRequestCELJoin) that verifies join() works
end-to-end by checking the PipelineRun pod log output matches
the expected changed file path
Note: The e2e test uses regex matching (empty goldenFile param)
rather than golden file comparison, since the output is a single
dynamic file path that varies per test run.
Signed-off-by: Zaki Shaikh <zashaikh@redhat.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
zakisk
force-pushed
the
SRVKP-11940-support-string-ops-in-cel
branch
from
403f143 to
2067db9
Compare
|
/retest |
3 participants
📝 Description of the Change
Previously, CEL expressions in Pipelines-as-Code only had access to the core CEL operators, which limited users to basic comparisons and logical expressions. Functions like join(), replace(), substring(), and other string/list manipulation operations were unavailable, forcing users to work around these limitations.
This adds cel-go's ext.Strings() and ext.Lists() extensions to the CEL environment in pkg/cel/cel.go. This unlocks the full set of standard CEL string operations (join, replace, substring, split, trim, upperAscii, lowerAscii, etc.) and list operations, which can be used in both
on-cel-expressionannotations and{{ cel: }}template expressions.Changes:
Note: The e2e test uses regex matching (empty goldenFile param) rather than golden file comparison, since the output is a single dynamic file path that varies per test run.
🔗 Linked GitHub Issue
Fixes #
JIRA
https://redhat.atlassian.net/browse/SRVKP-11940
🧪 Testing Strategy
🤖 AI Assistance
AI assistance can be used for various tasks, such as code generation,
documentation, or testing.
Please indicate whether you have used AI assistance
for this PR and provide details if applicable.
Important
Slop will be simply rejected, if you are using AI assistance you need to make sure you
understand the code generated and that it meets the project's standards. you
need at least know how to run the code and deploy it (if needed). See
startpaac to make it easy
to deploy and test your code changes.
If the majority of the code in this PR was generated by an AI, please add a
Co-authored-bytrailer to your commit message.For example:
Co-authored-by: Claude noreply@anthropic.com
✅ Submitter Checklist
fix:,feat:) matches the "Type of Change" I selected above.make testandmake lintlocally to check for and fix anyissues. For an efficient workflow, I have considered installing
pre-commit and running
pre-commit installtoautomate these checks.